Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: dynamic font resizing functionality #570

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Conversation

klauer
Copy link
Contributor

@klauer klauer commented Aug 18, 2023

Description

Motivation and Context

New embedded positioner screens in #563 - @ZLLentz was aiming to have positioner readback values display in large form uniformly on all displays.

How Has This Been Tested?

A bit interactively and with the included unit test.
It'd be a good idea to do more interactive testing, I think.
I also updated our upstream test repository in the meantime with the same tweaks, so it's a lot easier to test there:

Where Has This Been Documented?

This PR.

Screenshots (if appropriate):

The readback widget has this applied - and the font fills it well:
image

@klauer
Copy link
Contributor Author

klauer commented Aug 18, 2023

Is this worth pursuing @ZLLentz or should we aim for a different approach?
Relevant commit for a convenient single-click: 476a146

@ZLLentz
Copy link
Member

ZLLentz commented Aug 18, 2023

I think this is worth pursuing, not just for the readback but for the other visible text fields in the positioner widget (both the compact version and the non-compact version)

My only critique is the lack of margins which plays poorly with the blue borders (but probably looks exactly correct for non-blue-border elements)

@klauer
Copy link
Contributor Author

klauer commented Aug 18, 2023

My only critique is the lack of margins which plays poorly with the blue borders (but probably looks exactly correct for non-blue-border elements)

Added a padding setting for that purpose ("pad_percent", proportional to widget size) in the patching method.

I'll try to extract this from the positioner PR now - apologies for the bumpy force pushes ahead

@@ -393,6 +393,18 @@ def unit_changed(self, new_unit):
if new_unit.lower() in EXPONENTIAL_UNITS and default:
self.displayFormat = DisplayFormat.Exponential

@Property(bool, "dynamicFontSize")
Copy link
Contributor Author

@klauer klauer Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used and probably should be removed
I erroneously thought that TyphosLabel et al were in the designer, but these are really used exclusively for auto-generated device displays (where we have the ophyd signal at instantiation time, etc)

@klauer klauer marked this pull request as ready for review August 18, 2023 20:51
@klauer klauer changed the title ENH/WIP: dynamic font resizing functionality ENH: dynamic font resizing functionality Aug 18, 2023
Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very 👍 on all of this
We can decide in a separate PR later how many and which widgets should get this applied to them inside the positioner widgets

Copy link
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this a lot, my testing of various random devices this was subtle but noticeable.

I do think this could be applied sparingly, not just for performance reasons, but also to be intentional with emphasis. (We could pick specific important fields to allow dynamic resizing on)

Do we think there's value in a ctrl + + / ctrl + - type of feature? Assuming it could be implemented without as bad of a performance penalty (maybe we don't have to hook paintEvent, but rather just scale font sizes up and down), that might provide users better control over sizes.

@klauer
Copy link
Contributor Author

klauer commented Aug 21, 2023

Do we think there's value in a ctrl + + / ctrl + - type of feature? Assuming it could be implemented without as bad of a performance penalty (maybe we don't have to hook paintEvent, but rather just scale font sizes up and down), that might provide users better control over sizes.

I think that's a good idea, though rather hard to implement properly - one widget may resize as you had hoped and another may be clipped and unreadable.

I think context-sensitive menus in PyDM, allowing fine-grained font size adjustment on a per-widget basis, could be an option. Context menus are rather underutilized there in my opinion. Imagine an "Advanced" section that allows you to increase the (fixed) size of a given widget and/or increase the font size to match its size. I think of PyDM as an expert-facing app of sorts and giving more knobs to the user (so long as they're unobtrusive) could only be a positive thing.

@klauer klauer merged commit 191b0e4 into pcdshub:master Aug 21, 2023
7 of 9 checks passed
@klauer klauer deleted the dynamic_test branch August 21, 2023 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants